Skip to content

[Functional Tests] Use @kbn/test on Kibana CI#18967

Merged
rhoboat merged 22 commits intoelastic:masterfrom
rhoboat:archanid-18593
Jun 26, 2018
Merged

[Functional Tests] Use @kbn/test on Kibana CI#18967
rhoboat merged 22 commits intoelastic:masterfrom
rhoboat:archanid-18593

Conversation

@rhoboat
Copy link
Copy Markdown

@rhoboat rhoboat commented May 10, 2018

Addresses #18593

Background

We currently use the @kbn/test package on CI in one place: running X-Pack Functional, API, and SAML API tests. These run in succession with a single script (node scripts/functional_tests from x-pack/).

We'd like to use the package to run OSS Kibana tests on CI as well. Currently we run API tests in the test:api grunt task, and Selenium (functional) tests in the jenkins:selenium grunt task. We can unify these or keep them separate. (See #18593 for high-level details.)

Running from source/snapshot

One of the key requirements:

ES should always run from source when on CI. It should run from snapshot by default (for dev).

Now pass in --esFrom=<source|snapshot> default: snapshot

  • Check for invalid cli args
  • Replace api tests
  • Replace selenium tests
  • Jest tests

How to test:

Unit tests

There are Jest tests.

Integration tests

Look at the test run outputs in Jenkins.

  • Search for Running "run:functional_tests" (run) task in multijob-selenium task
  • Search for Running "run:api_integration_tests" (run) task in multijob-intake task

Then in the console output, just take a look at how Elasticsearch is started up (from source on CI), how Kibana is run with cli args, and how functional test runner runs.

Manual testing in development

# bail option not a boolean
node scripts/functional_tests --config test/functional/config.js --esFrom snapshot --debug --grep management --bail=peanut -- --server.maxPayloadBytes=1648576

# esFrom option not valid
node scripts/functional_tests --config test/functional/config.js --esFrom butter --debug --grep management --bail=peanut -- --server.maxPayloadBytes=1648576

# config option not a string
node scripts/functional_tests --config --esFrom source --debug --grep management --bail=peanut -- --server.maxPayloadBytes=1648576

# works with esFrom = source
node scripts/functional_tests --config test/functional/config.js --esFrom source --debug --grep management --bail -- --server.maxPayloadBytes=1648576

# server.<option> options not valid before the --
node scripts/functional_tests --config test/functional/config.js --esFrom source --debug --server.maxPayloadBytes=1648576 --grep management --bail=peanut -- --server.maxPayloadBytes=1648576

# works with multiple config options
node scripts/functional_tests --config test/functional/config.js --config test/api_integration/config.js --esFrom snapshot --debug --grep management --bail -- --server.maxPayloadBytes=1648576

# works with no config options
node scripts/functional_tests --esFrom snapshot --debug --grep management --bail -- --server.maxPayloadBytes=1648576

@rhoboat rhoboat changed the title Replace test:api with @kbn/test runTests [Functional Tests] Use @kbn/test on Kibana CI May 10, 2018
Comment thread tasks/config/run.js Outdated
Copy link
Copy Markdown
Author

@rhoboat rhoboat May 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Remove test:api related code.
  • Replace it with a call to @kbn/test code, scoped to just run api tests.
  • Run api_integration_tests using the scripts/functional_tests (which uses @kbn/test
    • Pass in just the config for api_integration tests
    • jenkins_config.js is built on the original with just a change to run ES from source

Alternative way to do this:

  • Call runTests directly from @kbn/test, though I'm not sure how to do this. If it needs to be done in a command-line way, then we already have it in scripts/functional_tests.
  • Allow test/api_integration/config.js to take in a from.
  • Or allow scripts/functional_tests to take in a --from.
    • This approach seems worth trying, for comparison.

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented May 10, 2018

@tylersmalley @azasypkin @spalger @epixa What do you think so far? I.e. Which of the 3 alternatives would you prefer?

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented May 10, 2018

jenkins test this

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@spalger
Copy link
Copy Markdown
Contributor

spalger commented May 10, 2018

What about a fourth option, the CI environment variable is always set when we run in CI, so we could just update https://github.com/elastic/kibana/blob/master/test/common/config.js#L21

    esTestCluster: {
      license: 'oss',
-     from: 'snapshot',
+     from: process.env.CI ? 'source' : 'snapshot',
      serverArgs: [
      ],
    },

@spalger
Copy link
Copy Markdown
Contributor

spalger commented May 10, 2018

Actually, relying on the CI environment variable makes it pretty tricky to run elasticsearch from source intentionally, and will require a good amount of digging into the code to figure out how.

I'm personally a fan of the --es-from command line flag, mostly because I like that it's documented in --help and that command line flags are normally how I tweak the behavior of a command line tool.

I just ran the x-pack/scripts/functional_tests_server script and was surprised by my inability to choose where es was built from. I tried --help, but it didn't offer any, and I would prefer that it did.

@tylersmalley
Copy link
Copy Markdown
Member

I am also +1 for the command line argument. Running from source is not something that we just want to do for Jenkins, it's also if you are working off an ES feature branch.

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented May 17, 2018

jenkins test this

@kimjoar kimjoar mentioned this pull request May 17, 2018
13 tasks
@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Copy link
Copy Markdown
Contributor

@spalger spalger May 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes seems really redundant.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just use options['es-from'] || 'snapshot'? This ternary will allow passing --es-from=foo and interpret it as --es-from=snapshot because any none "source" value is ignored.

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented May 21, 2018

jenkins, test this

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented May 22, 2018

hmm. this keeps failing

23:50:16 │ proc [ftr] └- ✖ fail: "management test large number of fields test_huge data should have expected number of fields"
23:50:16 │ proc [ftr] │ tryForTime timeout: Error: tryForTime timeout: Error: tryForTime timeout: [POST http://localhost:9515/session/ba2732fe4fefc2e1444fba16e5f86da7/element / {"using":"css selector","value":"[data-test-subj~="tab-count-indexedFields"]"}] no such element: Unable to locate element: {"method":"css selector","selector":"[data-test-subj~="tab-count-indexedFields"]"}

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented May 22, 2018

jenkins test this

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@rhoboat rhoboat added the WIP Work in progress label May 22, 2018
@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented Jun 20, 2018

@kobelb @azasypkin I've made the default log level debug for the tasks in tasks/config/run.js

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented Jun 20, 2018

@azasypkin yes, indeed, was waiting for that dev.mode flag removal. 🎉

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented Jun 22, 2018

@kobelb I took care of your comments, i think you're out on vacation...

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented Jun 22, 2018

jenkins test this

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@rhoboat
Copy link
Copy Markdown
Author

rhoboat commented Jun 26, 2018

jenkins, test this

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Copy link
Copy Markdown
Contributor

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks great! I've run all possible functional/integration tests I could think of in Kibana and x-pack, everything worked flawlessly.

--help Display this menu and exit.
--config <file> Pass in a config. Can pass in multiple configs.
--esFrom <snapshot|source> Build Elasticsearch from source or run from snapshot. Default: snapshot
--kibana-install-dir <dir> Run Kibana from existing install directory instead of from source.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it's the only parameter that uses dashes now, maybe we can make it --kibanaInsallDir just for consistency?

Copy link
Copy Markdown
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - the improved test coverage is great!

@rhoboat rhoboat merged commit b03243f into elastic:master Jun 26, 2018
mattapperson pushed a commit that referenced this pull request Jun 28, 2018
* Replace test:api with @kbn/test runTests

* Improve CLI help menu 🆘

* Use --es-from

* Replace jenkins:selenium with kbn-test

* Validate cli args, fixing test in the process

* Clean up some stuff

* Code review fixes

* Explanation for collectCliArgs

* Remove exit codes, they're useless anyway.

* Make markdown vis test pass with dev_mode setting

* Tests

* Remove unneeded export

* Code review: move console logging up to cli.js

* Code review: refactor startServers and runTests to take single options collection

* Code review: Remove all things I am sure we do not use

* Improve tests

* Code review fixes

* Pass created log to runFtr, runElasticsearch, runKibanaServer

* Update --es-from option to --esFrom
@rhoboat rhoboat deleted the archanid-18593 branch June 28, 2018 16:56
rhoboat pushed a commit that referenced this pull request Jun 29, 2018
* [Functional Tests] Use @kbn/test on Kibana CI (#18967)

* Replace test:api with @kbn/test runTests

* Improve CLI help menu 🆘

* Use --es-from

* Replace jenkins:selenium with kbn-test

* Validate cli args, fixing test in the process

* Clean up some stuff

* Code review fixes

* Explanation for collectCliArgs

* Remove exit codes, they're useless anyway.

* Make markdown vis test pass with dev_mode setting

* Tests

* Remove unneeded export

* Code review: move console logging up to cli.js

* Code review: refactor startServers and runTests to take single options collection

* Code review: Remove all things I am sure we do not use

* Improve tests

* Code review fixes

* Pass created log to runFtr, runElasticsearch, runKibanaServer

* Update --es-from option to --esFrom

* Let dev server run from snapshot by default
rhoboat pushed a commit that referenced this pull request Jun 29, 2018
* [Functional Tests] Use @kbn/test on Kibana CI (#18967)

* Replace test:api with @kbn/test runTests

* Improve CLI help menu 🆘

* Use --es-from

* Replace jenkins:selenium with kbn-test

* Validate cli args, fixing test in the process

* Clean up some stuff

* Code review fixes

* Explanation for collectCliArgs

* Remove exit codes, they're useless anyway.

* Make markdown vis test pass with dev_mode setting

* Tests

* Remove unneeded export

* Code review: move console logging up to cli.js

* Code review: refactor startServers and runTests to take single options collection

* Code review: Remove all things I am sure we do not use

* Improve tests

* Code review fixes

* Pass created log to runFtr, runElasticsearch, runKibanaServer

* Update --es-from option to --esFrom

* Still need dev_mode setting in 6.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t//

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants